Run as user#102
Conversation
soenkeliebau
left a comment
There was a problem hiding this comment.
I've left a few comments/questions that we could discuss, but overall I'd be happy for this to go in as is as well.
Nit: when running clippy from the nightly toolchain there are two complaints.
| WantedBy=multi-user.target"#} | ||
| ), | ||
| )] | ||
| fn create_unit_from_pod( |
There was a problem hiding this comment.
This is fabulous and I'm fine with merging as is but it's not testing any of the error cases like a wrong user name
There was a problem hiding this comment.
It is correct that nearly every line on the happy path is tested but the exception path is ignored. It is definitely worth to test the error cases, too. But my goal is to parse and validate the PodSpec in a first step and then pass this valid structure around where further validation is not necessary. This keeps the tests in the downstream modules comprehensible. Otherwise it would be necessary to define a full PodSpec to test the regular expression for user names and another full PodSpec to test the order of the environment variables and so on. If everything is tested where it belongs to then the tests can be reduced to 3-liners.
… user is not set in the systemd unit
TODOs:
Every state and module extracts the parts of the PodSpec it needs. Currently the
systemdunitgathers the information for the service user. This user is not known in the other states and modules. Therefore thecreating_configstate cannot assign the ownership of the configuration directory to this user. I suggest to gather all necessary information from the PodSpec in the first (new) state, store all information in a struct in thePodStateand disregard the PodSpec in all other states.